Add artifact relations tool and enrich fetch_artifacts with relation metadata#11
Open
Add artifact relations tool and enrich fetch_artifacts with relation metadata#11
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new get_artifact_relations tool to the CodeAlive MCP server, enabling exploration of call graphs, inheritance hierarchies, and symbol references. Additionally, the fetch_artifacts tool has been updated to include call relations in its XML output and wrap source code within a new <content> element. Review feedback focuses on enhancing the robustness of XML generation by ensuring that potential null values from API responses are handled safely before calling html.escape and that all XML attributes are consistently escaped to prevent malformed output.
Comment on lines
+109
to
+121
| source_id = html.escape(data.get("sourceIdentifier", "")) | ||
| profile = html.escape(data.get("profile", "")) | ||
| found = data.get("found", False) | ||
|
|
||
| # Map profile back to MCP-friendly name | ||
| mcp_profile = profile | ||
| for mcp_name, api_name in PROFILE_MAP.items(): | ||
| if api_name == profile: | ||
| mcp_profile = mcp_name | ||
| break | ||
|
|
||
| if not found: | ||
| return f'<artifact_relations sourceIdentifier="{source_id}" profile="{mcp_profile}" found="false"/>' |
There was a problem hiding this comment.
There are two issues in this block:
- Potential Crash:
html.escapewill raise anAttributeErrorif the API returnsnullforsourceIdentifierorprofile, becausedict.get(key, default)returnsNoneif the key is present with anullvalue. Usingdata.get("key") or ""ensures a string is always passed. - Logic Error: The profile mapping logic compares the escaped
profilestring with the rawapi_namefromPROFILE_MAP. If the profile name contains characters that require escaping (e.g.,&), the comparison will fail. Mapping should occur on the raw value before escaping for XML.
Suggested change
| source_id = html.escape(data.get("sourceIdentifier", "")) | |
| profile = html.escape(data.get("profile", "")) | |
| found = data.get("found", False) | |
| # Map profile back to MCP-friendly name | |
| mcp_profile = profile | |
| for mcp_name, api_name in PROFILE_MAP.items(): | |
| if api_name == profile: | |
| mcp_profile = mcp_name | |
| break | |
| if not found: | |
| return f'<artifact_relations sourceIdentifier="{source_id}" profile="{mcp_profile}" found="false"/>' | |
| raw_source_id = data.get("sourceIdentifier") or "" | |
| raw_profile = data.get("profile") or "" | |
| found = data.get("found", False) | |
| # Map profile back to MCP-friendly name | |
| mcp_profile = raw_profile | |
| for mcp_name, api_name in PROFILE_MAP.items(): | |
| if api_name == raw_profile: | |
| mcp_profile = mcp_name | |
| break | |
| source_id_attr = html.escape(raw_source_id) | |
| profile_attr = html.escape(mcp_profile) | |
| if not found: | |
| return f'<artifact_relations sourceIdentifier="{source_id_attr}" profile="{profile_attr}" found="false"/>' |
Comment on lines
+145
to
+154
| attrs = [f'identifier="{html.escape(item.get("identifier", ""))}"'] | ||
|
|
||
| file_path = item.get("filePath") | ||
| if file_path is not None: | ||
| attrs.append(f'filePath="{html.escape(file_path)}"') | ||
|
|
||
| start_line = item.get("startLine") | ||
| if start_line is not None: | ||
| attrs.append(f'startLine="{start_line}"') | ||
|
|
There was a problem hiding this comment.
Similar to previous comments, html.escape will crash if identifier is null. Additionally, startLine should be escaped for XML robustness.
Suggested change
| attrs = [f'identifier="{html.escape(item.get("identifier", ""))}"'] | |
| file_path = item.get("filePath") | |
| if file_path is not None: | |
| attrs.append(f'filePath="{html.escape(file_path)}"') | |
| start_line = item.get("startLine") | |
| if start_line is not None: | |
| attrs.append(f'startLine="{start_line}"') | |
| for item in group.get("items", []): | |
| attrs = [f'identifier="{html.escape(item.get("identifier") or "")}"'] | |
| file_path = item.get("filePath") | |
| if file_path is not None: | |
| attrs.append(f'filePath="{html.escape(file_path)}"') | |
| start_line = item.get("startLine") | |
| if start_line is not None: | |
| attrs.append(f'startLine="{html.escape(str(start_line))}"') |
Comment on lines
+183
to
+192
| call_id = html.escape(call.get("identifier", "")) | ||
| summary = call.get("summary") | ||
| if summary is not None: | ||
| call_elements.append( | ||
| f' <call identifier="{call_id}" summary="{html.escape(summary)}"/>' | ||
| ) | ||
| else: | ||
| call_elements.append(f' <call identifier="{call_id}"/>') | ||
|
|
||
| parts.append(f' <{tag} count="{count}">') |
There was a problem hiding this comment.
Ensure that identifier is not null before passing it to html.escape to avoid an AttributeError. Also, escape the count attribute to ensure the generated XML is robust against unexpected API response types.
Suggested change
| call_id = html.escape(call.get("identifier", "")) | |
| summary = call.get("summary") | |
| if summary is not None: | |
| call_elements.append( | |
| f' <call identifier="{call_id}" summary="{html.escape(summary)}"/>' | |
| ) | |
| else: | |
| call_elements.append(f' <call identifier="{call_id}"/>') | |
| parts.append(f' <{tag} count="{count}">') | |
| call_id = html.escape(call.get("identifier") or "") | |
| summary = call.get("summary") | |
| if summary is not None: | |
| call_elements.append( | |
| f' <call identifier="{call_id}" summary="{html.escape(summary)}"/>' | |
| ) | |
| else: | |
| call_elements.append(f' <call identifier="{call_id}"/>') | |
| parts.append(f' <{tag} count="{html.escape(str(count))}">') |
Comment on lines
+134
to
+142
| total_count = group.get("totalCount", 0) | ||
| returned_count = group.get("returnedCount", 0) | ||
| truncated = str(group.get("truncated", False)).lower() | ||
|
|
||
| xml_parts.append( | ||
| f' <relation_group type="{html.escape(mcp_type)}" ' | ||
| f'totalCount="{total_count}" returnedCount="{returned_count}" ' | ||
| f'truncated="{truncated}">' | ||
| ) |
There was a problem hiding this comment.
This block has robustness issues:
- If
totalCountorreturnedCountarenullin the JSON response,group.get(..., 0)will returnNone, which might cause issues later or result in the string"None"in the XML. - If
truncatedisnull,str(None).lower()results in"none", which is not a valid XML boolean value (true/false). - Attributes should be escaped to ensure the XML remains well-formed even if the API returns unexpected string values.
Suggested change
| total_count = group.get("totalCount", 0) | |
| returned_count = group.get("returnedCount", 0) | |
| truncated = str(group.get("truncated", False)).lower() | |
| xml_parts.append( | |
| f' <relation_group type="{html.escape(mcp_type)}" ' | |
| f'totalCount="{total_count}" returnedCount="{returned_count}" ' | |
| f'truncated="{truncated}">' | |
| ) | |
| total_count = group.get("totalCount") or 0 | |
| returned_count = group.get("returnedCount") or 0 | |
| truncated = str(bool(group.get("truncated"))).lower() | |
| xml_parts.append( | |
| f' <relation_group type="{html.escape(mcp_type)}" ' | |
| f'totalCount="{html.escape(str(total_count))}" returnedCount="{html.escape(str(returned_count))}" ' | |
| f'truncated="{truncated}">' | |
| ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a new
get_artifact_relationsMCP tool and enrichesfetch_artifactswith optional call relation metadata.